Add keyed ILogger which forwards to ScriptHost when possible #11398
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a mechanism for WebHost services to forward their logs to the active ScriptHost logger when available, falling back to the WebHost logger when no ScriptHost is active. The implementation uses WeakReference to avoid memory leaks and automatically tracks changes to the active ScriptHost.
Key changes:
- Introduces ForwardingLogger classes that automatically switch between WebHost and ScriptHost loggers
- Adds a ForwardingLoggerAttribute for dependency injection that uses keyed services
- Updates IScriptHostManager interface to expose Services property for accessing ScriptHost services
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/WebJobs.Script/Host/IScriptHostManager.cs | Adds Services property and enables nullable reference types |
| src/WebJobs.Script/Diagnostics/ForwardingLogger.cs | Core forwarding logger implementation with weak references |
| src/WebJobs.Script/Diagnostics/ForwardingLoggerFactory.cs | Factory for creating forwarding loggers |
| src/WebJobs.Script/Diagnostics/ForwardingLoggerAttribute.cs | Attribute for keyed service injection |
| src/WebJobs.Script/Extensions/ScriptLoggingBuilderExtensions.cs | Extension methods for registering forwarding logger services |
| src/WebJobs.Script/Diagnostics/HealthChecks/TelemetryHealthCheckPublisher.cs | Example usage of ForwardingLogger attribute |
| src/WebJobs.Script.WebHost/Program.cs | Registers forwarding logger in web host startup |
| src/WebJobs.Script.WebHost/Diagnostics/ILoggingBuilderExtensions.cs | Formatting and return type improvements |
| test/ files | Comprehensive unit tests for all new functionality |
Add mechanism to forward logs from WebHost to ScriptHost
jviau
force-pushed
the
u/jviau/log-forwarder
branch
from
910a686 to
46b1a61
Compare
| /// <summary> | ||
| /// Gets the current <see cref="IServiceProvider"/> for the active Script Host. | ||
| /// </summary> | ||
| IServiceProvider? Services { get; } |
There was a problem hiding this comment.
I have found a couple times recently that having access to IServiceProvider here is useful. Not sure if there is a reason we didn't expose it in the past?
jviau
force-pushed
the
u/jviau/log-forwarder
branch
from
69cd8f7 to
9d5193b
Compare
|
|
Issue describing the changes in this PR
resolves #11400
Pull request checklist
IMPORTANT: Currently, changes must be backported to the
in-procbranch to be included in Core Tools and non-Flex deployments.in-procbranch is not requiredrelease_notes.md-- TODOAdditional information
This PR introduces a mechanism where WebHost services can opt into having their logs forwarded to the active ScriptHost
ILoggerwhen it is available. When a ScriptHost is not available, logs will go to the WebHostILogger. The implementation automatically tracks changes to active ScriptHost, while avoiding memory leaks through the use ofWeakReference<T>(allowed active ScriptHostILoggerto be cleaned up on ScriptHost shutdown).Usage
Simply add
[ForwardingLogger]attribute toILogger<T>orILoggerFactoryimport:TODO
Evaluate how/if the added keyed services are being proxied to the ScriptHost servicesI have excludedILoggerFactoryandILogger<>from being copied to script host service container. They were not actually used as ScriptHost overrode with their own, but the copying process did fail validation.